Passing s and d (which are int file descriptors on Unix) to my_error with the ER_ERROR_ON_RENAME format specifier will cause a segmentation fault/crash because ER_ERROR_ON_RENAME expects string arguments (const char*). They should be replaced with the actual file names.

If aria_backup_start was never called (e.g., because another plugin's backup_start failed earlier in plugin_foreach_with_mask), aria_backup will be nullptr. When plugin_foreach_with_mask subsequently calls backup_end for all plugins, aria_backup_end will dereference aria_backup without a null check, causing a server crash. A null check should be added.

int aria_backup_end(THD *thd, bool abort) noexcept
{
  if (!aria_backup)
    return 0;
  int ret_val= aria_backup->end(thd, abort);
  aria_backup.reset();
  return ret_val;
}

If backup_start was never called or failed, max_first_lsn is 0. In this case, calling InnoDB_backup::end will trigger assertions like ut_ad(log_sys.archive) which can fail and crash the debug server. We should return early if max_first_lsn is 0.

  int end(THD *thd, bool abort) noexcept
  {
    int fail= 0;
    log_sys.latch.wr_lock();
    if (!max_first_lsn)
    {
      log_sys.latch.wr_unlock();
      return 0;
    }

If backup_start was never called or failed, max_first_lsn is 0. In this case, calling InnoDB_backup::fini will check the transaction state and trigger the assertion ut_ad("invalid state" == 0) because the transaction state is not TRX_STATE_BACKUP. We should return early if max_first_lsn is 0.

The parameters FINAL_INCLUDE, FINAL_EXCLUDE, and TARGET_DIR are interpolated into shell commands. To effectively prevent command injection, we should validate these inputs using a strict whitelist of allowed characters (such as alphanumeric characters and a limited set of safe symbols like '-', '_', '.') instead of relying on a blacklist or simple escaping.

Handling MARIADB_OPTS as a flat string will break if any option contains spaces or special characters (e.g., --password="my secret pass"). We should use a bash array instead.

We should append connection options to the MARIADB_OPTS array instead of concatenating them into a flat string to correctly handle arguments with spaces or special characters.

        --user=*|--password=*|--host=*|--port=*|--socket=*)
            # Connection options (long form) - pass through to mariadb client
            MARIADB_OPTS+=("$1")
            shift
            ;;
        -u|-p|-h|-P|-S)
            # Short form. If the next argv looks like a value (not another
            # flag), consume it as well — supports both `-u USER` and bare
            # `-p` (which mariadb client treats as a password prompt).
            if [[ -n "${2-}" && "$2" != -* ]]; then
                MARIADB_OPTS+=("$1" "$2")
                shift 2
            else
                MARIADB_OPTS+=("$1")
                shift
            fi
            ;;
        -u*|-p*|-h*|-P*|-S*)
            # Bundled short form: -uUSER, -pSECRET, -hHOST, ...
            MARIADB_OPTS+=("$1")
            shift
            ;;
        --defaults-file=*|--defaults-extra-file=*)
            # Config file options - pass through to mariadb client
            MARIADB_OPTS+=("$1")
            shift
            ;;

To be fully standard and portable, open with O_DIRECTORY should explicitly include the access mode O_RDONLY.

To be fully standard and portable, open with O_DIRECTORY should explicitly include the access mode O_RDONLY.

    , datadir_fd(open(maria_data_root, O_RDONLY | O_DIRECTORY))

Can this be implemented in a way that is compatible with the .combinations logic?

I would like most of --suite=mariabackup to be run both with the genuine mariadb-backup and with the BACKUP SERVER based wrapper scripts. Currently, this is the only test that exercises the wrappers, and it fails to test the original executables to demonstrate compatibility between them and the wrappers.

I’m not sure if this should be in the current scope. We don’t have any MDEV-38362 streaming backup yet. What should be more in the scope is demonstrating that the mbstream wrapper script is syntactically compatible with its namesake utility.

Can we write this for the POSIX shell? Bourne Again Shell (bash) is not available in all environments, for good reasons: reduced resource usage as well as attack surface; remember https://en.wikipedia.org/wiki/Shellshock_(software_bug)

These are necessary but not sufficient rules. mbstream -xp4 would be a valid invocation of the original utility (equivalent to -x -p4), but not of GNU tar.

I did test that mbstream indeed treats anything after -p as a single argument. For example, -p4t and -p4x would report the following, respectively:

Warning: option 'parallel': signed value 4398046511104 adjusted to 2147483647
Unknown suffix 'x' used for variable 'parallel' (value '4x'). Legal suffix characters are: K, M, G, T, P, E
mbstream: Error while setting value '4x' to 'parallel'

Please add a test file that covers both the real mbstream and this wrapper. Both with some invalid and valid invocation. For example, mbstream -t is not allowed, while tar -t is.